Skip to content

Add JFR GCTLABConfiguration Event#24307

Open
adpopescu wants to merge 1 commit into
eclipse-openj9:masterfrom
adpopescu:jfr-gc-tlab-configuration-event
Open

Add JFR GCTLABConfiguration Event#24307
adpopescu wants to merge 1 commit into
eclipse-openj9:masterfrom
adpopescu:jfr-gc-tlab-configuration-event

Conversation

@adpopescu

Copy link
Copy Markdown
Contributor

This change add support for the JFR GC TLAB Configuration Event.

@adpopescu adpopescu force-pushed the jfr-gc-tlab-configuration-event branch 2 times, most recently from 6845d90 to 4385528 Compare July 8, 2026 15:38
@adpopescu

Copy link
Copy Markdown
Contributor Author

@tajila @amicic please review.

@thallium

thallium commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

As stated in #24194, we are moving away from deferring every-chunk event generation to writing chunk. Instead, in JFR v2 we should respond to emitEvent() immediately by storing the event in JFR buffer just like regular events.

@adpopescu

Copy link
Copy Markdown
Contributor Author

@thallium @tajila Is there something I'm missing? From what I can tell I'm doing exactly that? I have a requestGCTLABConfiguration function that is called from requestEvent? It reserves the buffer and stores the event in there? The data is generated at initialize anyway since it's a one time event.

@thallium

Copy link
Copy Markdown
Contributor

It's a one time event by default but technically a user can configure it to be requested multiple times per chunk and the current implementation can't handle that. So ideally we want to response to a request immediately (i.e. the event in JFR buffer should contain actual event data) rather than delay it to chunk writing. There might be other concerns as well? @tajila

@adpopescu

adpopescu commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

hmm ok, none of the other configuration based ones seem to be doing that right now? So is the plan to implement those in that way as well? Or can those events not be configured to be requested multiple times per chunk?

@adpopescu

Copy link
Copy Markdown
Contributor Author

It did seem strange to me that there was essentially a placeholder for many of the events instead of using the actual sizes of the events... I assumed it was just the way we were dealing with the config one time based ones.

@thallium

Copy link
Copy Markdown
Contributor

Yes most of the everyChunk/beginChunk/endChunk events haven't been updated. TheadDump event has been updated: #24216, and I implemented ThreadAllocationStatistics event in the preferred way in #23155.

@adpopescu

adpopescu commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Those are not good examples for this... those are events that should be able to be requested at any time since their contents change... so yes, you fill them in at the time of the actual request. The data for this event does not change. So it only needs to be filed in once during initializeJFRConstantEvents.

@thallium

Copy link
Copy Markdown
Contributor

Sorry I didn't notice this is a constant event... yes it's less of a trouble for constant events so I'll wait for Tobi's opinion.

@adpopescu

Copy link
Copy Markdown
Contributor Author

Ah ok, we could move all the constant events into this type of structure as well? But I don't think it would really save us all that much? In fact it may slow things down since the event data is refilled every time.

@tajila

tajila commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

In general we are trying to generate all events in the local/global buffers at the time they are requested for the reasons Gengchen mentioned. However, given these events are constant then its probably fine for now, we can keep it as is.

@tajila

tajila commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

jenkinst test sanity.functional xlinux jdk17

@tajila

tajila commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

jenkins test sanity.functional xlinux jdk17

@tajila

tajila commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

jenkins compile win jdk17

@tajila

tajila commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

jenkins test sanity.functional alinux64 jdk17,jdk21

@tajila

tajila commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

please rebase these changes

This change add support for the JFR GC TLAB Configuration Event.

Signed-off-by: Adrian Popescu <adpopescu@ibm.com>
@adpopescu adpopescu force-pushed the jfr-gc-tlab-configuration-event branch from 4385528 to a44a2cc Compare July 14, 2026 14:47
@adpopescu

Copy link
Copy Markdown
Contributor Author

should be good now @tajila

tlabConfiguration->minTLABSize = mmFuncs->j9gc_get_tlh_minimum_size(vm);
/* The closest OMR equivalent to tlabRefillWasteLimit is the
* initial value of abandonSize in TLHAllocationSupport::refresh(), which
* evaluates to max(tlhMinimumSize, tlhInitialSize / 2) on a fresh thread.

@amicic amicic Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a correct interpretation - the waste is roughly 1/2 of refresh size, because current refresh size typically dominates over initial/minimum

The hard part is that the refresh size is local to each allocating thread

For fast allocating threads refresh size is close to tlhMaximumSize, and for slow allocating it's close to tlhMinimumSize

If we want to generalize, we can say refresh is tlhMaximumSize across VM, in which case the waste limit is 1/2 of that. More correct would be to account for the largest refresh of all allocating threads (what would require to walk over all threads at the moment of the API invocation).

It may be relevant for X platforms where tlhMaximumSize defaults to 1MB, and is not always reached (by refresh). On other platforms were it defaults to 128KB it's pretty much always reached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants